Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix "exists" matching as many times as there are duplicates in an array of values #26

Merged
merged 1 commit into from
Jun 24, 2019

Conversation

scottinet
Copy link
Contributor

@scottinet scottinet commented Jun 21, 2019

Description

Operands (and, or, bool) work by maintaining a counter of different conditions that must all be met before a subfilter can match.

That counter should only be decremented once per tested condition on any given subfilter.
But when testing an array of values with exists, the exists matcher decrements the counter without verifying if the condition has already been verified.

For instance, the following condition: exists: foo["bar"] matches 2 times if the following object is tested: {"foo": ["bar", "bar"]}
And, if it's used in an operand waiting for, say, 2 different conditions, then that keyword can validate the entire operand by itself, even if the other condition is not verified.

This bug has been solved by making the exists matcher test only a set of unique values extracted from the array.

I checked the other matchers and they seem unaffected by this bug.

fix #24

How to review

I dusted off the code a bit while I was reading the test files, so there are a lot of cosmetic changes that can be ignored in the unit test files.
Just ignore them, nothing changed in these files, except for one new test that I pointed with a comment so that you can easily find where it is.

});
});

it('(see issue #24) should handle duplicates gracefully', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the new test, other modifications in test files are only cosmetic.

@jenow jenow merged commit f823f37 into 1-dev Jun 24, 2019
@jenow jenow deleted the fix-24 branch June 24, 2019 09:32
@scottinet scottinet mentioned this pull request Jun 28, 2019
scottinet added a commit that referenced this pull request Jun 28, 2019
# [1.2.2](https://github.com/kuzzleio/koncorde/releases/tag/1.2.2) (2019-06-28)


#### Bug fixes

- [ [#26](#26) ] Fix "exists" matching as many times as there are duplicates in an array of values   ([scottinet](https://github.com/scottinet))

#### Others

- [ [#27](#27) ] Dependencies update   ([scottinet](https://github.com/scottinet))
- [ [#23](#23) ] Fix dead links in the README file   ([Aschen](https://github.com/Aschen))
---
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants